-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Added custom theme builder #161
base: master
Are you sure you want to change the base?
feat: Added custom theme builder #161
Conversation
Here is a demo of how it works currently: IMG_1616.mov |
Also sorry for forgetting to include my .gitignore for the .vscode extension. I will add it next commit. |
I am actually going to configure this. I want to be able to do a rgb color panel like this: https://www.w3schools.com/colors/colors_picker.asp |
nice work, just two comments: dependencies.lock.esp32s3 shouldn't be in .gitignore, at least in the PR. |
|
||
|
||
void generate_color_string(char* buffer, size_t size, const char* color_name, int color_value) { | ||
snprintf(buffer, size, "%s{%d}", color_name, color_value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually 'const' int params where possible.
We usually JADE_ASSERT pointers and sizes/length params to functions.
We usually JADE_ASSERT the result of snprintf calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotchya! will do. I will look for other places in the code to use JADE_ASSERT as well.
snprintf(buffer, size, "%s{%d}", color_name, color_value); | ||
} | ||
|
||
void handle_color_activity(const char* color_name, int* color, gui_view_node_t* color_text, gui_activity_t* parent_act) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert comments as above
int32_t ev_id; | ||
while (gui_activity_wait_event(act, GUI_EVENT, ESP_EVENT_ANY_ID, NULL, &ev_id, NULL, 0)) { | ||
if (ev_id == GUI_WHEEL_LEFT_EVENT) { | ||
*color = (*color - 8 + 256) % 256; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe have a 'const uint16_t limit = UINT8_MAX + 1' rather than 256 or a #define ?
Maybe #define the '8' step size also.
// Update color string and repaint | ||
generate_color_string(color_str, sizeof(color_str), color_name, *color); | ||
gui_update_text(color_c_text, color_str); | ||
gui_repaint(color_c_text->parent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gui_repaint() shouldn't be needed - it should do this itself.
Nice idea! Some small comments in the code. Some thoughts (just ideas!):
Just to be clear, the above are just musings/ideas, not hard 'you must...' comments. ;-) |
This was created because of a feature request here #110
This theme builder is not fully complete which is why I am opening this as a draft. The theme builder will allow you to build a custom theme by entering RGB codes into the create custom theme window. Your custom theme will be one of the selections of possible themes.
This code is not super optimized yet and I hope it doesn't stray to far away from regular conventions. I would love feed back if you could offer any.
:)